-
Notifications
You must be signed in to change notification settings - Fork 364
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: [M3-8103] - Refactor Image Create and Add Tags #10471
feat: [M3-8103] - Refactor Image Create and Add Tags #10471
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The primary changes are in here 🚨
Coverage Report: ✅ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new create flow looks good and the code is very readable and clean 🧼.
✅ Image create page matches mockups
✅ Confirmed I can create an Image from a Linode and Disk and select Label, Tags and Description
✅ CI passing
getOptionDisabled={ | ||
grants | ||
? (linode) => | ||
grants.linode.some( | ||
(grant) => | ||
grant.id === linode.id && | ||
grant.permissions === 'read_only' | ||
) | ||
: undefined | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of disabling these options would it make sense to filter them out via optionsFilter
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That was the existing behavior. We can do that, but in my opinion, the better UX would be to show the items and explain why they are disabled. That's why I added the helper text of You can only create Images from Linodes you have read/write access to.
for this case. Happy to revert if we think this is worse UX
I have a cafe item today that will touch on this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, nice work @bnussman-akamai!
Went through the flow a couple times and everything worked as expected, double checked small screen layout, thatdouble checked the payload contained my cloud-init selection, description, tags, etc.
The test changes and additions also look fantastic -- thanks for keeping those in mind!
Description 📝
Refactors the Image Create page and adds the ability to add tags during creation. This PR does not touch the upload page. These changes will not be feature flagged.
Changes 🔄
react-hook-form
✏️Preview 📷
How to test 🧪
As an Author I have considered 🤔